Allow dropping `dyn` trait object's principal #114679

compiler-errors · 2023-08-10T00:43:45Z

Allows us to cast from dyn Trait + Send to dyn Send, dropping the trait object's principal trait. This is distinct from trait upcasting, since it's trivial even if we were to totally rip out upcasting support -- it uses the same codepath as dropping auto traits, like dyn Trait + Send to dyn Trait, which is currently allowed without feature gates on stable.

This seems like something that would be nice to have, if anything, just for consistency. Nominating for T-lang discussion and FCP for that reason. I don't personally think this needs a feature gate, but that is also I guess up for debate!

This was originally noticed by lcnr in #113393 (comment).

r? types on the impl

rustbot · 2023-08-10T00:43:53Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

compiler/rustc_trait_selection/src/traits/select/confirmation.rs

lcnr · 2023-08-10T07:35:12Z

r=me after you answer my question and add some (run-pass) tests 😁

It might make sense to still stabilize this with trait upcasting because unlike dyn Trait + Send to dyn Trait, it does impact the vtable?

Does the current impl simply transmute the trait vtable to the empty auto trait vtable?

compiler-errors · 2023-08-10T16:00:38Z

Does the current impl simply transmute the trait vtable to the empty auto trait vtable?

Yes, it does -- it just reinterprets the vtable as being its common prefix. There's no new vtable being produced.

lcnr · 2023-08-11T08:42:41Z

@rustbot author

compiler-errors · 2023-08-15T04:20:32Z

compiler/rustc_codegen_ssa/src/base.rs

@@ -154,7 +154,9 @@ pub fn unsized_info<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
        ) if src_dyn_kind == target_dyn_kind => {
            let old_info =
                old_info.expect("unsized_info: missing old info for trait upcasting coercion");
-            if data_a.principal_def_id() == data_b.principal_def_id() {
+            if data_a.principal_def_id() == data_b.principal_def_id()
+                || data_b.principal().is_none()


@bjorn3: if this PR ends up being accepted, the change will need to be made to cg-clif as well. should I do that in this PR, or in a separate one?

Doing it in the same PR would be nice I think. You can change

rust/compiler/rustc_codegen_cranelift/src/unsize.rs

Lines 34 to 37 in a32978a

if data_a.principal_def_id() == data_b.principal_def_id() {

// A NOP cast that doesn't actually change anything, should be allowed even with invalid vtables.

return old_info;

}

the same way as you did here.

pnkfelix · 2023-08-22T18:16:02Z

Discussed in T-lang triage meeting; our conclusion was that if immediate stabilization is desired, then @rust-lang/types is the right team to own authoring a stabilization report (or, if the effort for a stabilization report is unwarranted, then the PR author can instead add a feature gate), so this can remain S-waiting-on-team, but the relevant team is now T-types.

@rustbot label: +T-types +I-types-nominated

lcnr · 2023-09-11T14:43:38Z

discussed in the t-types meeting: waiting on stabilization report + FCP by @compiler-errors

compiler-errors · 2024-01-22T19:15:26Z

I actually don't see any case where someone would want this. It's cute from the type system perspective, but I don't have the time to convince T-lang that this has a real world use case.

… r=compiler-errors Allow dropping dyn principal Revival of rust-lang#126660, which was a revival of rust-lang#114679. Fixes rust-lang#126313. Allows dropping principal when coercing trait objects, e.g. `dyn Debug + Send` -> `dyn Send`. cc `@compiler-errors` `@Jules-Bertholet` r? `@lcnr`

Rollup merge of rust-lang#131857 - WaffleLapkin:dyn-drop-principal-3, r=compiler-errors Allow dropping dyn principal Revival of rust-lang#126660, which was a revival of rust-lang#114679. Fixes rust-lang#126313. Allows dropping principal when coercing trait objects, e.g. `dyn Debug + Send` -> `dyn Send`. cc `@compiler-errors` `@Jules-Bertholet` r? `@lcnr`

compiler-errors added T-lang Relevant to the language team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. I-lang-nominated Nominated for discussion during a lang team meeting. labels Aug 10, 2023

rustbot assigned jackh726 Aug 10, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 10, 2023

lcnr reviewed Aug 10, 2023

View reviewed changes

compiler/rustc_trait_selection/src/traits/select/confirmation.rs Show resolved Hide resolved

lcnr assigned lcnr and unassigned jackh726 Aug 10, 2023

lcnr mentioned this pull request Aug 10, 2023

Tracking issue for dyn upcasting coercion #65991

Open

21 tasks

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 11, 2023

Allow dropping dyn principal

abbdc26

rust-cloud-vms bot force-pushed the dyn-principal branch from 018e98c to abbdc26 Compare August 15, 2023 04:18

compiler-errors added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 15, 2023

compiler-errors commented Aug 15, 2023

View reviewed changes

rustbot added I-types-nominated Nominated for discussion during a types team meeting. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Aug 22, 2023

oli-obk removed T-lang Relevant to the language team, which will review and decide on the PR/issue. I-lang-nominated Nominated for discussion during a lang team meeting. labels Sep 6, 2023

lcnr removed the I-types-nominated Nominated for discussion during a types team meeting. label Sep 11, 2023

compiler-errors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Sep 21, 2023

compiler-errors closed this Jan 22, 2024

compiler-errors deleted the dyn-principal branch June 12, 2024 10:02

compiler-errors mentioned this pull request Jun 12, 2024

trait Trait: Send: upcasting from dyn Trait to dyn Send does not work #126313

Closed

Jules-Bertholet mentioned this pull request Jun 19, 2024

Allow dropping dyn Trait principal #126660

Closed

WaffleLapkin mentioned this pull request Oct 17, 2024

Allow dropping dyn principal #131857

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow dropping `dyn` trait object's principal #114679

Allow dropping `dyn` trait object's principal #114679

compiler-errors commented Aug 10, 2023

rustbot commented Aug 10, 2023

lcnr commented Aug 10, 2023

compiler-errors commented Aug 10, 2023

lcnr commented Aug 11, 2023

compiler-errors Aug 15, 2023

bjorn3 Aug 15, 2023

pnkfelix commented Aug 22, 2023 •

edited

Loading

lcnr commented Sep 11, 2023

compiler-errors commented Jan 22, 2024

	if data_a.principal_def_id() == data_b.principal_def_id() {
	// A NOP cast that doesn't actually change anything, should be allowed even with invalid vtables.
	return old_info;
	}

Allow dropping dyn trait object's principal #114679

Allow dropping dyn trait object's principal #114679

Conversation

compiler-errors commented Aug 10, 2023

rustbot commented Aug 10, 2023

lcnr commented Aug 10, 2023

compiler-errors commented Aug 10, 2023

lcnr commented Aug 11, 2023

compiler-errors Aug 15, 2023

Choose a reason for hiding this comment

bjorn3 Aug 15, 2023

Choose a reason for hiding this comment

pnkfelix commented Aug 22, 2023 • edited Loading

lcnr commented Sep 11, 2023

compiler-errors commented Jan 22, 2024

Allow dropping `dyn` trait object's principal #114679

Allow dropping `dyn` trait object's principal #114679

pnkfelix commented Aug 22, 2023 •

edited

Loading